Skip to content

fix(onboarding): Handle repo selection race with background link_all_repos#111716

Merged
jaydgoss merged 14 commits intomasterfrom
jaygoss/fix-repo-selection-race-with-link-all-repos
Mar 30, 2026
Merged

fix(onboarding): Handle repo selection race with background link_all_repos#111716
jaydgoss merged 14 commits intomasterfrom
jaygoss/fix-repo-selection-race-with-link-all-repos

Conversation

@jaydgoss
Copy link
Copy Markdown
Member

Fix repo selection in the SCM onboarding connect step to handle the race with the background link_all_repos task.

After a GitHub integration is installed, link_all_repos runs async and registers all repos. The old approach cached existing repos on mount and checked that cache before deciding to POST. This broke because the cache was often stale (task still running) and the repos endpoint is paginated (specific repo might not be on page 1).

New approach:

  • GET first with a targeted query filtered by repo name (sidesteps pagination)
  • POST only if the repo doesn't exist yet

The old code also hid (soft-deleted) repo records when the user deselected or switched repos. This made sense when repos were manually registered one at a time, but with link_all_repos auto-registering everything, hiding repos on deselect was counterproductive -- it removed records the background task created, causing them to disappear from the integration settings page. Repo selection is now just a context pointer with no side effects on the repo records themselves.

Also switches to queryClient.fetchQuery + fetchDataQuery for the imperative GET instead of deprecated Client.requestPromise.

Replaces #111699 (closed due to CODEOWNERS auto-review noise from base change).

@jaydgoss jaydgoss requested a review from a team as a code owner March 27, 2026 16:01
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 27, 2026
},
},
];
const [matches] = await queryClient.fetchQuery({
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this different from our useApiQuery?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat, useApiQuery uses fetchDataQuery under the hood (passed to useQuery as a queryFn), this is the imperative equivalent. We need the GET inside a callback, so the hook doesn't make sense here.

  • user selects a repo
  • check if it exists (imperative GET)
  • conditionally POST

@jaydgoss jaydgoss force-pushed the jaygoss/fix-repo-selection-race-with-link-all-repos branch from 67c18fd to 41763b5 Compare March 30, 2026 17:54
@jaydgoss jaydgoss force-pushed the jaygoss/vdy-24-scm-platform-features-step-ui-polish-and-analytics-pass branch from 491f83c to c2bb8d3 Compare March 30, 2026 18:06
@jaydgoss jaydgoss force-pushed the jaygoss/fix-repo-selection-race-with-link-all-repos branch from 41763b5 to 58a0241 Compare March 30, 2026 18:06
Base automatically changed from jaygoss/vdy-24-scm-platform-features-step-ui-polish-and-analytics-pass to master March 30, 2026 20:22
@jaydgoss jaydgoss requested a review from a team as a code owner March 30, 2026 20:22
jaydgoss added 14 commits March 30, 2026 15:23
…repos

After a GitHub integration is installed, link_all_repos runs async and
may still be populating repos when the user reaches the repo selector.
The previous approach cached existing repos on mount and looked them
up before deciding whether to POST, but the cache was often stale or
paginated, causing selection to fail silently.

Now always POST first. If the repo already exists (background task or
previous session created it), the POST fails and we fall back to a
targeted GET filtered by repo name to find the existing record. This
avoids both the stale cache and pagination issues.
Flip the order: do a targeted GET filtered by repo name first, and
only POST if the repo doesn't exist yet. This avoids expected 400s
showing up in devtools and Sentry when the background link_all_repos
task has already created the repo.
Replace direct Client.requestPromise usage with fetchMutation which
wraps the same call but is the recommended API.
fetchMutation only accepts PUT/POST/DELETE methods. Use the shared
QUERY_API_CLIENT instance directly for the imperative GET call.
Replace deprecated QUERY_API_CLIENT.requestPromise with the sanctioned
queryClient.fetchQuery + fetchDataQuery pattern for imperative GET
requests outside of React Query hooks.
Use ApiQueryKey type with getApiUrl for proper URL typing, satisfying
the queryKey constraint on queryClient.fetchQuery.
With link_all_repos running in the background after integration
install, all repos are automatically registered in Sentry. The
hide-on-deselect and cleanup-on-switch logic was designed for manual
repo registration and is now counterproductive — it removes repo
records that the background task created, causing them to disappear
from settings. Selection is now just a context pointer.
Rewrite tests to match the GET-first-then-POST approach and removal
of repo hide/cleanup logic. Remove tests for cleanup-on-switch and
hide-on-remove since those behaviors no longer exist.
Match on r.name === repo.identifier instead of externalSlug because
externalSlug varies by provider (GitLab uses a numeric project ID,
VSTS uses external_id). Repository.name is consistently the full
repo path across all providers.
…rage

Remove unnecessary async from handleRemove test to fix require-await
lint error. Rename misleading test name to accurately reflect that only
POST fails (GET succeeds with empty results). Add dedicated test for GET
failure scenario. Simplify busy state test by removing waitFor dance
that couldn't observe the intermediate state.
@jaydgoss jaydgoss force-pushed the jaygoss/fix-repo-selection-race-with-link-all-repos branch from 58a0241 to b289d7b Compare March 30, 2026 20:25
@jaydgoss jaydgoss merged commit fe7b09d into master Mar 30, 2026
74 checks passed
@jaydgoss jaydgoss deleted the jaygoss/fix-repo-selection-race-with-link-all-repos branch March 30, 2026 20:39
dashed pushed a commit that referenced this pull request Apr 1, 2026
…repos (#111716)

Fix repo selection in the SCM onboarding connect step to handle the race
with the background `link_all_repos` task.

After a GitHub integration is installed, `link_all_repos` runs async and
registers all repos. The old approach cached existing repos on mount and
checked that cache before deciding to POST. This broke because the cache
was often stale (task still running) and the repos endpoint is paginated
(specific repo might not be on page 1).

New approach:
- GET first with a targeted query filtered by repo name (sidesteps
pagination)
- POST only if the repo doesn't exist yet

The old code also hid (soft-deleted) repo records when the user
deselected or switched repos. This made sense when repos were manually
registered one at a time, but with `link_all_repos` auto-registering
everything, hiding repos on deselect was counterproductive -- it removed
records the background task created, causing them to disappear from the
integration settings page. Repo selection is now just a context pointer
with no side effects on the repo records themselves.

Also switches to `queryClient.fetchQuery` + `fetchDataQuery` for the
imperative GET instead of deprecated `Client.requestPromise`.

Replaces #111699 (closed due to CODEOWNERS auto-review noise from base
change).
@github-actions github-actions bot locked and limited conversation to collaborators Apr 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants